fix: updated mlx related pkgs + end condition for stopping model request loop#111
fix: updated mlx related pkgs + end condition for stopping model request loop#111
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR updates many pinned Python dependencies and lockfiles (notably bumping mlx, mlx-lm, transformers, replacing requests/urllib3 with httpcore/httpx, and adding several tokenizer/CLI packages), adds a preinstall script, and changes container/model files (qwen base image). It removes the explicit model download flow: deletes Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/stack/requirements/app-server/pylock.app-server.toml`:
- Around line 233-245: The call to snapshot_download is receiving the removed
parameter "local_dir_use_symlinks" (it's set to False at the earlier assignment
and ends up in kwargs_dict passed to snapshot_download), which will raise a
TypeError with huggingface-hub 1.8.0; remove that key from kwargs_dict before
calling snapshot_download (e.g., pop/remove "local_dir_use_symlinks" from
kwargs_dict in the fallback/throttled-worker download path so the
snapshot_download call no longer receives the unexpected keyword).
In `@server/stack/venvstacks.toml`:
- Line 29: The exception handlers around snapshot_download in
server/hf_downloader.py (the except block catching
requests.exceptions.HTTPError/ConnectionError/Timeout at lines ~120-127) must be
updated to handle the newer huggingface-hub v1.8.0 behavior: replace or extend
the caught exceptions to include httpx.HTTPError and the Hugging Face
hub-specific HTTP error (HfHubHttpError / HfHubHTTPError) and add the
corresponding imports, while keeping any existing requests-based catches as a
fallback so both old and new backends are supported; update the except clause(s)
to catch (httpx.HTTPError, HfHubHttpError) alongside the existing
requests.exceptions.*.
In `@tiles/src/runtime/mlx.rs`:
- Line 553: Remove the unconditional debug print that uses println! of the
variable content (the line containing println!("{}", content)); either delete
that line or replace it with a conditional debug log (e.g., tracing::debug! or
log::debug!) behind a feature flag so the full content is not printed to stdout
in production. Locate the println! call in tiles/src/runtime/mlx.rs where
content is printed and switch to a non-stdout, conditional logging mechanism or
remove it entirely.
- Around line 557-559: The sentinel "END" return in extract_reply (used in
mlx.rs) is causing downstream logic to treat it as a real assistant message
(affecting g_reply emptiness checks and causing "END" to be stored in
conversations/DB); change extract_reply to return Option<String> (None for the
loop-termination case) or otherwise stop returning the literal "END", and update
convert_to_chat_response and all callers (the code that checks g_reply, the
conversation push logic around conversations and DB persistence) to explicitly
handle None as "terminate/skip storing" while preserving empty-string replies as
valid responses so the existing g_reply.is_empty() check still works. Ensure
references: extract_reply, convert_to_chat_response, g_reply, conversations, and
the DB persist path are updated to treat the termination signal separately from
a real reply.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9716b2ef-9554-478b-869d-7f91a9ee317a
⛔ Files ignored due to path filters (1)
server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
server/pyproject.tomlserver/stack/requirements/app-server/packages-app-server.txtserver/stack/requirements/app-server/pylock.app-server.meta.jsonserver/stack/requirements/app-server/pylock.app-server.tomlserver/stack/requirements/app-server/requirements-app-server.inserver/stack/requirements/framework-mlx/packages-framework-mlx.txtserver/stack/requirements/framework-mlx/pylock.framework-mlx.meta.jsonserver/stack/requirements/framework-mlx/pylock.framework-mlx.tomlserver/stack/requirements/framework-mlx/requirements-framework-mlx.inserver/stack/venvstacks.tomltiles/src/runtime/mlx.rs
| httpcore==1.0.9 | ||
| httpx==0.28.1 | ||
| huggingface-hub==1.8.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'import requests|requests\.exceptions|requests\.(get|post)\s*=|snapshot_download\s*\(|HfHubHttpError|set_(async_)?client_factory' server/hf_downloader.py server/throttled_download_worker.pyRepository: tilesprivacy/tiles
Length of output: 2917
🏁 Script executed:
cat -n server/stack/requirements/app-server/packages-app-server.txtRepository: tilesprivacy/tiles
Length of output: 1515
🏁 Script executed:
# Check if requests appears anywhere in the requirements file
rg -n "requests" server/stack/requirements/app-server/packages-app-server.txtRepository: tilesprivacy/tiles
Length of output: 44
🌐 Web query:
huggingface-hub 1.8.0 dependencies requests httpx
💡 Result:
huggingface-hub 1.8.0 requires httpx as a core dependency (>=0.23.0, <1), but does not require requests. Since v1.0, the library has migrated from requests to httpx as its HTTP backend, dropping requests as a dependency. httpx is listed under core dependencies on PyPI, while requests is absent. It also appears as an optional dependency in some extras (e.g., oauth), but not required for base installation.
Citations:
- 1: https://pypi.org/project/huggingface-hub/
- 2: https://github.com/huggingface/huggingface_hub
- 3: https://archlinux.org/packages/extra/any/python-huggingface-hub/
- 4: https://huggingface.co/docs/huggingface_hub/en/concepts/migration
Update downloader code to use huggingface-hub v1's httpx-based HTTP backend instead of relying on requests.
requests is no longer in the requirements, but server/throttled_download_worker.py imports it (line 29), monkeypatches requests.get/post (lines 99–100), and catches requests.exceptions.* (lines 117, 132). Similarly, server/hf_downloader.py imports requests in its fallback path (line 116) and catches requests.exceptions.* (lines 122, 125, 128). Hub v1.8.0 migrated from requests to httpx and no longer includes requests as a dependency. The current monkeypatching approach will fail, and the custom throttling will not affect Hub traffic. Use set_client_factory() or set_async_client_factory() to hook into Hub's HTTP client instead.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- pre-install script to remove prev server folder by the installer - removed iroh error logs showing in release build - removed HF deps as downloader is done via RUST
1 participant
No description provided.